Skip to content

Improve BstIterator documentation and test coverage - #3630

Open
Nassim29-del wants to merge 1 commit into
iluwatar:masterfrom
Nassim29-del:improve-bst-iterator
Open

Nassim29-del wants to merge 1 commit into
iluwatar:masterfrom
Nassim29-del:improve-bst-iterator

Conversation

@Nassim29-del

Copy link
Copy Markdown

What this PR does

This PR makes small readability and documentation improvements to BstIterator. It does not change the traversal behavior.

Main changes

  • Removed the redundant throws NoSuchElementException from the next() signature, since it is an unchecked exception. It is still documented with @throws.
  • Moved the O(h) space explanation from the private helper to the class Javadoc.
  • Added constructor Javadoc, including that a null root represents an empty tree.
  • Documented that the tree must not be modified during iteration.
  • Replaced the parameter reassignment in pushPathToNextSmallest with a for loop.
  • Renamed the local variable next to current to avoid confusion with the next() method.

Verification

Ran ./mvnw -pl iterator -am verify, and the build passed.

- Remove redundant unchecked exception from the next() signature
- Move the O(h) space explanation to the class Javadoc
- Document null root handling and tree modification during iteration
- Replace parameter reassignment with a for loop
- Rename local variable next to current for clarity

No change to traversal behavior.
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

PR Summary

Updated the BstIterator to improve readability and documentation without altering traversal behavior. This change updates the class Javadoc to note O(h) space usage and the non-modification requirement during iteration, adds constructor Javadoc (null root represents an empty tree), refactors left-path pushing to use a for loop, renames the local variable next to current in next(), and removes the redundant unchecked exception from the next() signature while retaining @throws documentation. Also initializes pathStack at declaration.

Changes

File Summary
iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java Updated class Javadoc to clarify O(h) space usage and non-modification requirement; added constructor Javadoc noting null root represents an empty tree; refactored pushPathToNextSmallest to use a for loop over the left spine; renamed local variable next to current in next(); removed redundant unchecked exception from next() signature while keeping @throws documentation; initialized pathStack at declaration.

autogenerated by presubmit.ai

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM!

Review Summary

Commits Considered (1)
  • 576721a: Improve BstIterator documentation and test coverage

  • Remove redundant unchecked exception from the next() signature

  • Move the O(h) space explanation to the class Javadoc

  • Document null root handling and tree modification during iteration

  • Replace parameter reassignment with a for loop

  • Rename local variable next to current for clarity

No change to traversal behavior.

Files Processed (1)
  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java (1 hunk)
Actionable Comments (0)
Skipped Comments (6)
  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java [19-19]

    maintainability: "Initialize pathStack at declaration."

  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java [11-13]

    documentation: "Document O(h) space and mutation caveats."

  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java [21-25]

    documentation: "Constructor documentation for edge-case root."

  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java [35-39]

    enhancement: "Use for-loop for left spine traversal."

  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java [58-65]

    readability: "Rename local variable in next() for clarity."

  • iterator/src/main/java/com/iluwatar/iterator/bst/BstIterator.java [52-56]

    documentation: "Method Javadoc consistency for unchecked exception."

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant